Summary

Adds TUnit.FsCheck library with property-based testing support, extending the test discovery to support custom test attributes inheriting from BaseTestAttribute.

Critical Issues

1. AOT Compatibility - Missing [DynamicallyAccessedMembers] Annotations ⚠️

The FsCheckPropertyTestExecutor.GetMethodInfo method uses reflection without proper AOT annotations (TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:48-84):

private static MethodInfo GetMethodInfo(Type classType, string methodName, ParameterMetadata[] parameters)
{
    var methods = classType
        .GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static)
        .Where(m => m.Name == methodName && m.GetParameters().Length == parameters.Length)
        .ToArray();
    // ...
}

Issue: The classType parameter lacks [DynamicallyAccessedMembers] annotation. Using UnconditionalSuppressMessage (line 43-45) suppresses warnings instead of properly preserving the necessary metadata for AOT.

Fix Required: Add proper annotation:

private static MethodInfo GetMethodInfo(
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | 
                                 DynamicallyAccessedMemberTypes.NonPublicMethods)] 
    Type classType, 
    string methodName, 
    ParameterMetadata[] parameters)

Remove the UnconditionalSuppressMessage attributes once proper annotations are in place.

2. Snapshot Tests Not Verified ⚠️

The PR checklist shows snapshot tests are unchecked. Since this PR modifies the source generator (TestMetadataGenerator.cs), you must:

1. Run TUnit.Core.SourceGenerator.Tests
2. Review any .received.txt files generated
3. Accept them as .verified.txt if the changes are intentional
4. Commit the updated .verified.txt files

Per CLAUDE.md Rule #2: "Changes to source generator output or public APIs require running snapshot tests. Commit .verified.txt files. NEVER commit .received.txt."

Suggestions

Dual-Mode Implementation ✅

Good work! Both paths are correctly updated:

• Source generator: TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs:39-97 (new GetCustomTestMethodMetadata)
• Reflection: TUnit.Engine/Discovery/ReflectionTestDataCollector.cs:1028-1041 (new IsTestMethod checks BaseTestAttribute)

The logic properly mirrors between both modes.

Performance Note

The new GetCustomTestMethodMetadata method iterates through all method attributes and checks base types in a loop. This is in the hot path (test discovery). Consider if this could be optimized, though it may be acceptable given discovery is a one-time operation.

Previous Review Status

No previous comments found.

Verdict

⚠️ REQUEST CHANGES - Critical AOT compatibility and snapshot test issues must be addressed before merge.

Example failing property output

Summary

This PR adds a new TUnit.FsCheck library that provides property-based testing support using FsCheck, allowing tests to be run with automatically generated input values.

Critical Issues

1. AOT Compatibility - Missing [DynamicallyAccessedMembers] Annotations ⚠️

Location: TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:44-84

The GetMethodInfo method uses reflection to discover methods without proper AOT annotations:

[UnconditionalSuppressMessage("Trimming", "IL2070", Justification = "FsCheck requires reflection")]
private static MethodInfo GetMethodInfo(Type classType, string methodName, ParameterMetadata[] parameters)
{
    var methods = classType
        .GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static)
        .Where(m => m.Name == methodName && m.GetParameters().Length == parameters.Length)
        .ToArray();
    // ...
}

Issue: Per CRITICAL RULE #5 in CLAUDE.md: "All code must work with Native AOT. Annotate reflection with [DynamicallyAccessedMembers]."

Using UnconditionalSuppressMessage hides the warning instead of properly preserving metadata for AOT compilation. This will cause runtime failures when published with Native AOT.

Required Fix:

private static MethodInfo GetMethodInfo(
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | 
                                 DynamicallyAccessedMemberTypes.NonPublicMethods)] 
    Type classType, 
    string methodName, 
    ParameterMetadata[] parameters)
{
    // Remove UnconditionalSuppressMessage attribute
    // ...
}

The same issue affects other methods that use reflection via MakeGenericMethod (lines 158-209). These also need proper annotations or should be marked with RequiresDynamicCode if they genuinely cannot work with AOT.

2. Blocking on Async in Hot Path 🚫

Location: TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:467-483

private static void HandleVoidResult(object? result)
{
    switch (result)
    {
        case Task task:
            task.GetAwaiter().GetResult();  // ❌ BLOCKING
            break;
        case ValueTask valueTask:
            valueTask.GetAwaiter().GetResult();  // ❌ BLOCKING
            break;
    }
}

private static bool HandleResult(object? result)
{
    switch (result)
    {
        case Task<bool> taskBool:
            return taskBool.GetAwaiter().GetResult();  // ❌ BLOCKING
        // ...
    }
}

Issue: Per CRITICAL RULE in CLAUDE.md: "NEVER block on async - No .Result or .GetAwaiter().GetResult()."

This violates TUnit's async-first architecture and can cause deadlocks in certain synchronization contexts.

Required Fix: The ExecuteTest method signature should be async:

public async ValueTask ExecuteTest(TestContext context, Func<ValueTask> action)
{
    // ... setup code ...
    await RunPropertyCheckAsync(methodInfo, classInstance, config);
}

Suggestions

1. Consider Performance Impact

The FsCheck integration uses reflection heavily in the test execution path. While property-based tests run multiple iterations (default 100), consider if there are opportunities to cache MethodInfo or generic method references to reduce repeated reflection overhead.

2. Error Message Formatting

The FormatCounterexample method (lines 291-340) does good work parsing FsCheck's output. Nice attention to user experience!

3. Snapshot Tests

I don't see any changes to source generator output or public API test surface in this PR. The changes appear to be purely additive (new library). Snapshot test verification may not be required unless there were generator changes not shown in the diff.

Previous Review Status

The previous review (by github-actions) raised the same AOT compatibility concern. It appears unaddressed in the current state of the PR.

Verdict

⚠️ REQUEST CHANGES - Two critical issues must be fixed:

1. Add proper [DynamicallyAccessedMembers] annotations for AOT compatibility
2. Remove blocking calls on async code (.GetAwaiter().GetResult())

These violations directly conflict with TUnit's CRITICAL RULES #5 (AOT Compatible) and the principle "NEVER block on async".

I realise that I made this way more complicated that it needed to be 😅 should be a bit simpler now

Summary

Adds TUnit.FsCheck library for property-based testing integration with FsCheck.

Critical Issues

1. AOT Compatibility Violation - Missing Proper Annotations ⚠️

Location: TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:43-86

The code uses UnconditionalSuppressMessage to hide AOT warnings instead of properly annotating reflection usage:

[UnconditionalSuppressMessage("Trimming", "IL2070", Justification = "FsCheck requires reflection")]
private static MethodInfo GetMethodInfo(
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
    Type classType,
    // ...

Issues:

1. The annotation only includes PublicMethods, but line 53 calls GetMethods with BindingFlags.NonPublic, which requires DynamicallyAccessedMemberTypes.NonPublicMethods
2. Using UnconditionalSuppressMessage hides the warning instead of fixing the root cause

Per CLAUDE.md Rule #5: "AOT Compatible - All code must work with Native AOT. Annotate reflection with [DynamicallyAccessedMembers]."

Required Fix:

private static MethodInfo GetMethodInfo(
    [DynamicallyAccessedMembers(
        DynamicallyAccessedMemberTypes.PublicMethods | 
        DynamicallyAccessedMemberTypes.NonPublicMethods)] 
    Type classType,
    string methodName,
    ParameterMetadata[] parameters)
{
    // Remove the UnconditionalSuppressMessage attribute
    var methods = classType.GetMethods(
        BindingFlags.Public | BindingFlags.NonPublic | 
        BindingFlags.Instance | BindingFlags.Static)
    // ...
}

2. FsCheck Library Requires Reflection - AOT Compatibility Concern

Location: TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:115-127

[UnconditionalSuppressMessage("Trimming", "IL2060", Justification = "FsCheck requires reflection")]
[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "FsCheck requires dynamic code")]
private static void RunPropertyCheck(MethodInfo methodInfo, object classInstance, Config config)
{
    Check.Method(config, methodInfo, classInstance);
}

Issue: FsCheck's Check.Method fundamentally requires reflection and dynamic code generation. This means TUnit.FsCheck cannot work with Native AOT.

Required Action: Since this library cannot be made AOT-compatible due to FsCheck's requirements:

1. Add [RequiresDynamicCode] and [RequiresUnreferencedCode] attributes to the entire FsCheckPropertyTestExecutor class
2. Document in the library README/docs that TUnit.FsCheck is not compatible with Native AOT
3. Consider whether this should block the PR, given TUnit's CRITICAL RULE Or assertion conditions #5

This is an architectural decision for the maintainer: Is it acceptable to ship an optional package that violates the AOT compatibility rule?

Suggestions

Missing Tests

The PR doesn't include any unit tests for the new library. Consider adding:

• Tests verifying FsCheck integration works correctly
• Tests for error message formatting (FormatCounterexample, ParseTupleValues)
• Tests for configuration mapping

Example Project Cleanup

TUnit.Example.FsCheck.TestProject/TUnit.Example.FsCheck.TestProject.sln appears unnecessary - the example project should just be part of the main solution (TUnit.sln already includes it).

Previous Review Status

Previous reviews by github-actions bot raised the same AOT compatibility concerns. The author mentioned simplifying the implementation, but the current diff shows the same code with the same issues.

Verdict

⚠️ REQUEST CHANGES - Critical AOT compatibility violations must be addressed:

1. Fix the incomplete DynamicallyAccessedMembers annotation in GetMethodInfo
2. Acknowledge that FsCheck fundamentally cannot work with Native AOT and decide if this is acceptable for an optional TUnit package
3. If proceeding, properly annotate the class with [RequiresDynamicCode] and [RequiresUnreferencedCode] and document the AOT incompatibility

The maintainer needs to make a policy decision: Should TUnit ship packages that cannot support Native AOT, or is AOT compatibility a hard requirement even for optional extension packages?

Summary

Adds TUnit.FsCheck library for property-based testing using FsCheck, extending TUnit with a new optional package.

Critical Issues

None found ✅

Observations

AOT Compatibility - Properly Documented

The code correctly uses [DynamicallyAccessedMembers] on the GetMethodInfo method (TUnit.FsCheck/FsCheckPropertyTestExecutor.cs:44-46) and appropriately suppresses remaining warnings with justification. The documentation clearly states the library is not AOT-compatible due to FsCheck's requirements (docs/docs/examples/fscheck.md:143-144), which is an acceptable trade-off for an optional extension library.

TUnit Rules Compliance

• Rule Repeat attributes  #1 (Dual-Mode): ✅ Not applicable - this is an extension library using existing extensibility points, not modifying core discovery
• Rule Retry attribute  #2 (Snapshot Tests): ✅ Not applicable - no changes to source generator output or core public APIs
• Rule Or assertion conditions #5 (AOT): ✅ Properly annotated where possible, documented where not compatible

Design

This PR demonstrates good use of TUnit's extensibility model:

• Uses ITestRegisteredEventReceiver to inject custom executor
• Uses IDataSourceAttribute interface appropriately
• No modifications to core TUnit engine or source generator
• Clean separation as optional package

Previous Review Comments

The github-actions bot raised concerns about AOT annotations, but these appear to have been addressed in the current code with proper [DynamicallyAccessedMembers] annotations present at line 44-46.

Verdict

✅ APPROVE - Well-designed extension library that properly uses TUnit's extensibility model. AOT limitations are acceptable and clearly documented for an optional property-based testing add-on.

Thanks Stu.

If you add this new project in GetPackageProjectsModule it'll automatically get packaged up and pushed out to Nuget 😄